-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use regex expression for authorization #32
Use regex expression for authorization #32
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic looks good, just a couple notes on the docs and I think this will be ready.
2172b7d
to
d4d5de7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
d4d5de7
to
68e922b
Compare
Rebased and pushed to be ready for merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me as well.
Any further input @willmostly ?
gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbAuthorizer.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbAuthorizer.java
Outdated
Show resolved
Hide resolved
Sorry for the delay @Chaho12 - I had a review pending that I didn't realize never got submitted |
Could you rebase and adjust to review comments please @Chaho12 ? Or maybe wait until the test refactor to use junit is in .. and then adapt. |
ced88bb
to
8a9da6d
Compare
8a9da6d
to
e772a0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
Use regex instead of contains to support more cases where more than 1 type of user members can exist and not all but some of the groups/members are allowed for User role.